Skip to content

fix(email): stop falling back to customer.io for transactional emails - #125322

Merged
trunk-io[bot] merged 2 commits into
posthog/messaging-suppression-hard-bounce-writersfrom
posthog/transactional-email-no-cio-fallback
Oct 29, 2026
Merged

trunk-io[bot] merged 2 commits into
posthog/messaging-suppression-hard-bounce-writersfrom
posthog/transactional-email-no-cio-fallback

Conversation

@mayteio

@mayteio mayteio commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Problem

  • Customer.io can't be turned off, because transactional emails that Workflows refuses still go out through it.

Changes

Note

Stacked on #124638, which limits transactional suppression to hard bounces. Without it, this PR would drop soft-bounced recipients.

  • A suppressed recipient is dropped, like an unreachable address. Customer.io no longer sends it.
  • Every other refusal retries on Workflows, then fails.
  • A retry stays unpinned when Workflows queued nothing, so turning the flag off still moves it to Customer.io.
  • A failed Workflows send retries for about 19 minutes, not 35 seconds.

Before:

flowchart LR
  A[send] --> W[Workflows]
  W -->|refused| C[Customer.io]
  W -->|uncertain| R[retry on Workflows]
  classDef phBlue fill:#1d4aff,stroke:#1d4aff,color:#fff;
  classDef phRed fill:#f54e00,stroke:#f54e00,color:#fff;
  class A,W,R phBlue;
  class C phRed;
Loading

After:

flowchart LR
  A[send] --> W[Workflows]
  W -->|suppressed or unreachable| D[drop]
  W -->|any other error| R[retry on Workflows]
  classDef phBlue fill:#1d4aff,stroke:#1d4aff,color:#fff;
  classDef phGray fill:#e5e7eb,stroke:#c7ccd1,color:#000;
  class A,W,R phBlue;
  class D phGray;
Loading

How did you test this code?

Test rationale: Routing cases expect a drop or failure, not Customer.io. New cases catch paused sending falling back, a needless pin, and a worker skipping the retry schedule.

Ran test_email.py locally.

Release status

  • No feature flag controls this change

Docs update

  • None.

🤖 Agent context

Autonomy: Human-driven (agent-assisted)

Agent: Claude Code, Opus 5.5 (claude-opus-5-5)

Skills: /writing-tests, /writing-code-comments, /writing-pr-descriptions, /stacking-prs

More context for AI agents
  • Session: PostHog Desktop task, PostHog MCP for event and flag data.
  • Decisions: The flag stays the kill switch while Customer.io stays configured, so only certain no-queue refusals skip the pin.
  • Decisions: Stacked on fix(messaging): send transactional workflow email unless the address hard-bounced #124638's stack because dropping suppressed recipients is only safe once suppression means hard bounce. Labeled cross-lane-change because the stack mixes Node and Python.
  • Follow-ups: Remove the Customer.io send path after a week with zero Customer.io sends.
  • Decisions: The app retry window stops near 19 minutes, because a late login code is useless. Billing retries for about 80 minutes in PostHog/billing#2630.
  • Read closely: _send_one, _workflows_queued_nothing and the retry branch in _send_email_now in posthog/email.py.

🤖 Generated with Claude Code


Created with PostHog Desktop

@mayteio mayteio self-assigned this Oct 29, 2026
@trunk-io

trunk-io Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

😎 Stack merged successfully - details.

@github-actions

github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Trunk lane — backend Python lane, mixes lanes

This PR is assigned to the backend Python lane. It runs backend Python tests and may merge in parallel with PRs in other lanes.

This PR changes Python or frontend code (posthog/email.py, posthog/test/test_email.py, products/messaging/backend/migrations/0011_messagesuppression_hard_bounced_at.py and 6 more) and Node or Rust code (nodejs/src/cdp/services/messaging/email-suppression.service.test.ts, nodejs/src/cdp/services/messaging/email-suppression.service.ts, nodejs/src/cdp/services/messaging/email-tracking.service.test.ts and 2 more) together. In the merge queue, every Node or Rust PR behind it in the same lane then runs the Django and frontend suites too, so the cross-lane check fails. Split it, or add the cross-lane-change label if the halves must land together.

⚠️ Duplication (Python) — 1 new duplicated block (worst 116 tokens)

New Python code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

First copy Second copy Lines Tokens
products/warehouse_sources/backend/temporal/data_imports/sources/incident_io/incident_io.py:17 products/warehouse_sources/backend/temporal/data_imports/sources/sendgrid/sendgrid.py:21 12 116
⚠️ Duplication (TypeScript) — 1 new duplicated block (worst 71 tokens)

New TypeScript code duplication introduced by this branch. Fails at 70+ tokens in app code, or 150+ tokens when both copies live in test files. Advisory while the gate proves itself: extract a shared helper instead of copying.

First copy Second copy Lines Tokens
products/web_analytics/frontend/pageReports/pageReportDetailLogic.ts:242 products/web_analytics/frontend/pageReports/pageReportDetailLogic.ts:256 12 71
🚨 Comment density — 11% of added code lines are comments (14 of 125)

This section warns when comments are more than 3% of the code lines a PR adds, and alerts above 6%. Before agent-assisted PRs, the typical share was about 2%. Only full-line comments count. Docstrings, generated files, snapshots, migrations, and workflow files are left out.

Comments that restate the code, record how the change came about, or narrate the next line add noise for the next reader. Keep the comments that explain a reason the code cannot show, and remove the rest. See .agents/skills/writing-code-comments/SKILL.md for the house rules.

Files with the most added comment lines:

File Comment lines Added lines
posthog/email.py 13 39
posthog/test/test_email.py 1 86

This check does not block merging. It updates on every push and clears when the share drops.

✅ Playwright — all passed

All tests passed.

View test results →

✅ Django migration risk — migration analysis complete

We've analyzed your migrations for potential risks.

Summary: 1 Safe | 0 Needs Review | 0 Blocked

✅ Safe

Brief or no lock, backwards compatible

messaging.0011_messagesuppression_hard_bounced_at
  └─ #1 ✅ AddField
     Adding nullable field requires brief lock
     model: messagesuppression, field: hard_bounced_at

📚 How to Deploy These Changes Safely

AddField:

This operation acquires a brief lock but doesn't rewrite the table.

Deployment uses lock timeouts with automatic retries, so lock contention will cause retries rather than connection pile-up.

Last updated: 2026-10-29 12:07 UTC (a06f866)

@trunk-io

trunk-io Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Static Badge   Static Badge   Static Badge

View Full Report ↗︎ ⋅ Docs

@mayteio
mayteio marked this pull request as ready for review October 29, 2026 10:37
@parameterai

parameterai Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Risk: Low · 1 low

This delta replaces Celery's default 5/10/20s autoretry for Workflows HTTP send failures with an explicit ~19-minute countdown schedule (10s→600s, 6 retries) driven by task.retry(...) in the worker, plus a _worker_send_task() helper refactor and matching tests. I traced the retry path end to end and found no new security issues: the schedule is bounded, retries replay under the same Workflows idempotency key derived from campaign_key+recipient (posthog/email.py:426-428), and Celery 5.6.3 (the pinned version, pyproject.toml) re-raises Retry untouched in its autoretry wrapper (except Retry: raise in celery/app/autoretry.py), so the explicit retry is not double-caught and cannot double-schedule or double-send.

Sentinel reviewed 52678e3 · Review settings

@pr-assigner-resolver-posthog
pr-assigner-resolver-posthog Bot requested a review from a team October 29, 2026 10:37
Comment thread posthog/email.py Resolved
@mayteio mayteio added the cross-lane-change Allow for cross lane (i.e. django + rust) changes to merge at once label Oct 29, 2026 — with PostHog
@mayteio
mayteio force-pushed the posthog/transactional-email-no-cio-fallback branch from 50602ed to 17dcb43 Compare October 29, 2026 11:10
@mayteio
mayteio changed the base branch from master to posthog/messaging-suppression-hard-bounce-writers October 29, 2026 11:10
@mayteio
mayteio added this pull request to stack #124639 October 29, 2026 11:10
@parameterai

parameterai Bot commented Oct 29, 2026

Copy link
Copy Markdown

On nodejs/src/cdp/services/messaging/email-suppression.service.ts:184, outside this diff.

🔵 One soft bounce resurrects a user-removed hard-bounce mark and drops password-reset email

An operator who re-enables a previously hard-bounced address (mailbox recreated, etc.) still loses transactional email to it, because the hard-bounce mark comes back on the first soft bounce — before any threshold is met.

How:

  1. An address hard-bounces; the row gets reason = 'Auto-suppressed after a hard bounce' and hard_bounced_at set.
  2. The operator removes the suppression: remove_suppression (products/messaging/backend/services/suppression.py:52-62) clears hard_bounced_at, resets the count and source, but leaves the reason text unchanged.
  3. The address soft-bounces once (e.g. a full inbox). The upsert here sets hard_bounced_at = COALESCE(NULL, NOW()) whenever source = 'BOUNCE' AND reason = ANY($5) — with no guard on suppressed/deleted and no threshold condition — so the cleared mark is back even though the row stays unsuppressed at count 1.
  4. When the count later reaches the soft-bounce threshold, blocks_transactional is true, and posthog/email.py:226 drops the send as recipient_suppressed with no retry and no Customer.io fallback — password resets and invoices to a working address silently stop.

The same resurrection exists in recordComplaints (email-suppression.service.ts:328-331): a complaint on a removed row re-marks it because the CASE reads the old source = 'BOUNCE' before re-keying to COMPLAINT.

Fix: clear the legacy reason text (or gate the legacy-reason branch on suppressed/deleted) in remove_suppression, so a deliberately re-enabled address starts from the clean slate the reset promises.


React with 👍 if useful or 👎 if not

A recipient on the Workflows suppression list is now dropped, like an unreachable address. Every other Workflows refusal retries on Workflows instead of sending through Customer.io. A retry stays unpinned when Workflows certainly queued nothing, so turning the flag off still moves it to Customer.io.

Generated-By: PostHog Desktop
Task-Id: 120fc7fd-3cc1-484a-99ee-da0e2036ca93
Customer.io no longer covers a Workflows outage, so a worker now retries a failed HTTP send after 10s, 30s, 1m, 2m, 5m and 10m instead of 5s, 10s and 20s. The window stops at about 19 minutes because a login code or password reset that arrives later is of no use. SMTP sends keep their existing retries.

Generated-By: PostHog Desktop
Task-Id: 120fc7fd-3cc1-484a-99ee-da0e2036ca93
@mayteio
mayteio force-pushed the posthog/transactional-email-no-cio-fallback branch from 52678e3 to a06f866 Compare October 29, 2026 12:03

mayteio commented Oct 29, 2026

Copy link
Copy Markdown
Contributor Author

Fixed in cfc9abd on #125374: remove_suppression now clears the reason along with hard_bounced_at, so a re-enabled address no longer matches the legacy hard-bounce reasons in recordTransientBounces or recordComplaints. The existing removal test now seeds a legacy reason and checks that it's cleared.

@trunk-io
trunk-io Bot merged commit 391e73d into master Oct 29, 2026
257 checks passed
@trunk-io
trunk-io Bot deleted the posthog/transactional-email-no-cio-fallback branch October 29, 2026 16:26
@deployment-status-posthog

deployment-status-posthog Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-10-29 17:20 UTC Run
prod-us ✅ Deployed 2026-10-29 17:42 UTC Run
prod-eu ✅ Deployed 2026-10-29 17:47 UTC Run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cross-lane-change Allow for cross lane (i.e. django + rust) changes to merge at once

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants